PERF: Native C++ parameter detection and execute pipeline - #549
PERF: Native C++ parameter detection and execute pipeline#549bewithgaurav wants to merge 38 commits into
Conversation
Move parameter type detection from Python into C++ using raw CPython type checks (PyLong_CheckExact, PyFloat_CheckExact, etc.). Merge the DetectParamTypes → BindParameters → SQLExecute pipeline into a single DDBCSQLExecuteFast call so ParamInfo never crosses the pybind11 boundary. - DetectParamTypes: handles int (range-detected), float, bool, str (unicode + geometry sniffing), bytes, datetime/date/time, Decimal (MONEY range + generic numeric), UUID, None, with fallback to string - SQLExecuteFast_wrap: single pipeline with GIL release, always uses SQLPrepare for parameterized queries - cursor.py: fast path routing when no setinputsizes overrides present; old DDBCSQLExecute path preserved for setinputsizes callers - Named constants: MAX_INLINE_CHAR, MAX_INLINE_BINARY, MAX_NUMERIC_PRECISION, MONEY/SMALLMONEY ranges, PARAM_C_TYPE_TEXT platform macro
- Add complete DAE (Data-At-Execution) loop to SQLExecuteFast_wrap: SQL_NEED_DATA → SQLParamData/SQLPutData for large str/bytes/binary, matching the existing SQLExecute_wrap logic exactly - Fix DAE type assignment: non-unicode DAE strings use SQL_C_CHAR (not PARAM_C_TYPE_TEXT which maps to SQL_C_WCHAR on macOS/Linux) - Fix MONEY range lower bound: use MONEY_MIN not SMALLMONEY_MIN so negative decimals in MONEY range bind as VARCHAR (matches Python path) - Raise TypeError for unknown param types instead of silent str conversion - Add SQLFreeStmt(SQL_RESET_PARAMS) to unbind after execute
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/pybind/ddbc_bindings.cppLines 242-250 242 SQLDescribeParamFunc SQLDescribeParam_ptr = nullptr;
243
244 namespace {
245
! 246
247 const char* GetSqlCTypeAsString(const SQLSMALLINT cType) {
248 switch (cType) {
249 STRINGIFY_FOR_CASE(SQL_C_CHAR);
250 STRINGIFY_FOR_CASE(SQL_C_WCHAR);Lines 571-580 571 dataPtr = sqlwcharBuffer->data();
572 bufferLength = sqlwcharBuffer->size() * sizeof(SQLWCHAR);
573 strLenOrIndPtr = AllocateParamBuffer<SQLLEN>(paramBuffers);
574 // Use explicit byte length instead of SQL_NTS so embedded NUL chars
! 575 // aren't treated as string terminators.
! 576 *strLenOrIndPtr = static_cast<SQLLEN>(sqlwcharBuffer->size() * sizeof(SQLWCHAR));
577 }
578 break;
579 }
580 case SQL_C_BIT: {Lines 713-721 713 dataPtr = static_cast<void*>(sqlTimePtr);
714 break;
715 }
716 case SQL_C_SS_TIMESTAMPOFFSET: {
! 717 py::object datetimeType = PyTypeCache::get_datetime_class_obj();
718 if (!py::isinstance(param, datetimeType)) {
719 ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex));
720 }
721 // Checking if the object has a timezoneLines 1770-1779 1770
1771 return ret;
1772 }
1773
! 1774 // LEGACY — slated for removal in a future optimization round.
! 1775 //
1776 // Executes the provided query using a ParamInfo list that Python already built,
1777 // rather than detecting parameter types in C++. Retained only for setinputsizes()
1778 // callers, whose explicit type overrides the native path does not yet honour.
1779 // Every parameter crosses the pybind11 boundary as a ParamInfo object here, whichLines 1914-1923 1914 continue;
1915 }
1916 if (PyUnicode_Check(pyObj)) {
1917 if (matchedInfo->paramCType == SQL_C_WCHAR) {
! 1918 std::u16string utf16 =
! 1919 borrow<py::str>(pyObj).cast<std::u16string>();
1920 rc = stream_dae_chunks(
1921 reinterpretU16stringAsSqlWChar(utf16),
1922 utf16.size() * sizeof(SQLWCHAR),
1923 putData);Lines 2009-2024 2009 py::list is_stmt_prepared,
2010 bool use_prepare,
2011 const py::dict& encoding_settings) {
2012 if (!statementHandle || !statementHandle->get()) {
! 2013 return SQL_INVALID_HANDLE;
! 2014 }
2015
2016 SQLHANDLE hStmt = statementHandle->get();
! 2017
! 2018 // Configure forward-only / read-only cursor (matches slow path semantics).
! 2019 if (SQLSetStmtAttr_ptr) {
! 2020 SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CURSOR_TYPE,
2021 (SQLPOINTER)SQL_CURSOR_FORWARD_ONLY, 0);
2022 SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CONCURRENCY,
2023 (SQLPOINTER)SQL_CONCUR_READ_ONLY, 0);
2024 }Lines 2028-2044 2028 // as ODBC's SQL_C_WCHAR. As a result, the only path that genuinely uses
2029 // byte-level character encoding is when the user explicitly opts in via
2030 // setencoding(..., ctype=mssql_python.SQL_CHAR) (which sends ctype=1, the
2031 // real ODBC SQL_CHAR). We default to utf-8 and only honor the dict's
! 2032 // encoding when ctype == 1 (real ODBC SQL_CHAR). Otherwise the user's
! 2033 // "encoding" value is meant for the wide-char path and we leave it alone.
! 2034 std::string charEncoding = "utf-8";
! 2035 if (encoding_settings.contains("ctype") && encoding_settings.contains("encoding")) {
! 2036 int ctype = encoding_settings["ctype"].cast<int>();
! 2037 if (ctype == SQL_C_CHAR /* real ODBC value: 1 */) {
! 2038 charEncoding = encoding_settings["encoding"].cast<std::string>();
! 2039 }
! 2040 }
2041
2042 // The cursor.py caller always passes a fresh `list(actual_params)` so this
2043 // function is free to mutate slots in place. Even so, every site below uses
2044 // PyList_SetItem (which decrefs the old slot before stealing the new ref),Lines 2059-2068 2059 if (!already_prepared) {
2060 if (use_prepare) {
2061 SQLWCHAR* queryPtr = reinterpretU16stringAsSqlWChar(query);
2062 {
! 2063 py::gil_scoped_release release;
! 2064 rc = SQLPrepare_ptr(hStmt, queryPtr, SQL_NTS);
2065 }
2066 if (!SQL_SUCCEEDED(rc)) return rc;
2067 statementHandle->clearDescribeCache();
2068 is_stmt_prepared[0] = py::bool_(true);Lines 2090-2100 2090 py::gil_scoped_release release;
2091 return SQLPutData_ptr(hStmt, data, len);
2092 };
2093 while (true) {
! 2094 {
! 2095 py::gil_scoped_release release;
! 2096 rc = SQLParamData_ptr(hStmt, ¶mToken);
2097 }
2098 if (rc != SQL_NEED_DATA) break;
2099
2100 const ParamInfo* matchedInfo = nullptr;Lines 2102-2118 2102 if (reinterpret_cast<SQLPOINTER>(const_cast<ParamInfo*>(&info)) == paramToken) {
2103 matchedInfo = &info;
2104 break;
2105 }
! 2106 }
! 2107 if (!matchedInfo) {
! 2108 ThrowStdException("SQLExecuteFast: unrecognized paramToken from SQLParamData");
! 2109 }
! 2110 PyObject* pyObj = matchedInfo->dataPtr.ptr();
2111 if (!pyObj || pyObj == Py_None) {
2112 py::gil_scoped_release release;
2113 SQLPutData_ptr(hStmt, nullptr, 0);
! 2114 continue;
2115 }
2116
2117 if (PyUnicode_Check(pyObj)) {
2118 if (matchedInfo->paramCType == SQL_C_WCHAR) {Lines 2116-2124 2116
2117 if (PyUnicode_Check(pyObj)) {
2118 if (matchedInfo->paramCType == SQL_C_WCHAR) {
2119 std::u16string u16 =
! 2120 borrow<py::str>(pyObj).cast<std::u16string>();
2121 rc = stream_dae_chunks(
2122 reinterpretU16stringAsSqlWChar(u16),
2123 u16.size() * sizeof(SQLWCHAR),
2124 putData);Lines 2136-2144 2136 } else if (PyBytes_Check(pyObj) || PyByteArray_Check(pyObj)) {
2137 // Handle bytes and bytearray separately — pybind11's bytes
2138 // caster does not safely convert bytearray.
2139 const char* dataPtr = nullptr;
! 2140 size_t totalBytes = 0;
2141 std::string bytesStorage; // lifetime must span the loop
2142
2143 if (PyBytes_Check(pyObj)) {
2144 bytesStorage = borrow<py::bytes>(pyObj);Lines 2152-2163 2152 totalBytes = bytesStorage.size();
2153 }
2154
2155 rc = stream_dae_chunks(dataPtr, totalBytes, putData);
! 2156 if (!SQL_SUCCEEDED(rc)) return rc;
! 2157 } else {
! 2158 ThrowStdException("SQLExecuteFast: DAE only supported for str or bytes");
! 2159 }
2160 }
2161 if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc;
2162 }Lines 2165-2173 2165
2166 // Unbind parameter buffers before they go out of scope.
2167 // Not called on error paths — diagnostics must remain readable.
2168 SQLRETURN exec_rc = rc;
! 2169 SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS);
2170 return exec_rc;
2171 }
2172
2173 SQLRETURN BindParameterArray(SqlHandle& handle, SQLHANDLE hStmt, const py::list& columnwise_params,Lines 2708-2716 2708
2709 // Get cached UUID class from module-level helper
2710 // This avoids static object destruction issues during
2711 // Python finalization
! 2712 py::object uuid_class = PyTypeCache::get_uuid_class_obj();
2713 // Get cached UUID class
2714
2715 for (size_t i = 0; i < paramSetSize; ++i) {
2716 const py::handle& element = columnValues[i];Lines 3797-3805 3797 int microseconds = dtoValue.fraction / 1000;
3798 py::object datetime_module = py::module_::import("datetime");
3799 py::object tzinfo = datetime_module.attr("timezone")(
3800 datetime_module.attr("timedelta")(py::arg("minutes") = totalMinutes));
! 3801 py::object py_dt = PyTypeCache::get_datetime_class_obj()(
3802 dtoValue.year, dtoValue.month, dtoValue.day, dtoValue.hour, dtoValue.minute,
3803 dtoValue.second, microseconds, tzinfo);
3804 row.append(py_dt);
3805 } else {mssql_python/pybind/param_detect.hppLines 382-391 382 info.columnSize = std::max<SQLULEN>(info.columnSize, time_len);
383 // PyList_SetItem (lowercase) decrefs the old slot before stealing the new
384 // reference; safe here because cursor.py already passed a fresh list copy.
385 if (PyList_SetItem(params, i, time_str) != 0) {
! 386 throw py::error_already_set();
! 387 }
388 continue;
389 }
390
391 // --- Decimal ---Lines 407-416 407 py::object digits_obj = steal(PyObject_GetAttrString(as_tuple_ptr.ptr(), "digits"));
408 if (!digits_obj) throw py::error_already_set();
409
410 if (!PyTuple_Check(digits_obj.ptr())) {
! 411 throw py::type_error("Decimal.as_tuple().digits must be a tuple");
! 412 }
413
414 Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj.ptr());
415 int exponent = static_cast<int>(PyLong_AsLong(exponent_obj.ptr()));
416 if (exponent == -1 && PyErr_Occurred()) throw py::error_already_set();Lines 465-474 465 PyObject* raw = formatted.release().ptr();
466 if (PyList_SetItem(params, i, raw) != 0) {
467 // PyList_SetItem steals (decrefs) the item even on failure,
468 // so raw is already freed — do NOT Py_DECREF here.
! 469 throw py::error_already_set();
! 470 }
471 continue;
472 }
473
474 // Build SQL_NUMERIC_STRUCT from the Decimal object. Store as a pybind11-castableLines 482-491 482 py::object numeric_obj = py::cast(nd);
483 PyObject* raw = numeric_obj.release().ptr();
484 if (PyList_SetItem(params, i, raw) != 0) {
485 // PyList_SetItem steals (decrefs) the item even on failure.
! 486 throw py::error_already_set();
! 487 }
488 continue;
489 }
490
491 // --- UUID ---Lines 499-508 499 info.columnSize = 16;
500 info.decimalDigits = 0;
501 if (PyList_SetItem(params, i, bytes_le) != 0) {
502 // PyList_SetItem steals (decrefs) the item even on failure.
! 503 throw py::error_already_set();
! 504 }
505 continue;
506 }
507
508 // --- Unknown type: raise TypeError (matches Python _map_sql_type) ---Lines 531-540 531 int sign_val = static_cast<int>(PyLong_AsLong(sign_obj.ptr()));
532 if (sign_val == -1 && PyErr_Occurred()) throw py::error_already_set();
533
534 if (!PyTuple_Check(digits)) {
! 535 throw py::type_error("Decimal.as_tuple().digits must be a tuple");
! 536 }
537
538 // SQL Server precision counts all stored decimal digits, while scale is just the
539 // fractional digits. A positive exponent moves trailing zeros into the integer part;
540 // a negative exponent means scale = -exponent and precision must still cover leadingLines 577-586 577 for (int j = 0; j < exponent; ++j) {
578 overflow |= mul10_add(0);
579 }
580 if (overflow != 0) {
! 581 throw py::value_error("Decimal magnitude exceeds the 16-byte SQL NUMERIC capacity");
! 582 }
583
584 NumericData nd;
585 nd.precision = static_cast<SQLCHAR>(precision);
586 nd.scale = static_cast<SQLSCHAR>(scale);mssql_python/pybind/py_type_cache.hppLines 44-55 44 // type detection in Python and can therefore reach here without the cache being warm;
45 // it can be dropped once that path is removed.
46 inline PyObject* get_cached_class(PyObject* cached, const char* module_name, const char* attr_name) {
47 if (cache_initialized && cached) return cached;
! 48 py::object mod = steal(PyImport_ImportModule(module_name));
! 49 if (!mod) return nullptr;
! 50 return PyObject_GetAttrString(mod.ptr(), attr_name);
! 51 }
52
53 // One-time init. Uses local py::objects so exception cleanup is automatic;
54 // only .release() into globals after ALL acquisitions succeed.
55 inline void initialize() {📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 75.1%
mssql_python.__init__.py: 77.3%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.4%
mssql_python.pybind.connection.connection.cpp: 83.7%
mssql_python.connection.py: 84.7%🔗 Quick Links
|
- Comment out use_prepare parameter name (C4100: unreferenced parameter) - Remove unused catch variable name (C4101: unreferenced local variable)
Add explicit null pointer and zero-length guards before memcpy in build_numeric_data to satisfy DevSkim code scanning rule DS121708.
…or attrs, parity test Six review fixes for SQLExecuteFast_wrap and DetectParamTypes: 1. Encoding key: read 'encoding' from settings dict (was 'charEncoding' which never matched). Only honor when ctype==SQL_C_CHAR so the default utf-16le doesn't corrupt SQL_C_CHAR DAE/inline byte paths. 2. Subclass support: PyLong_Check/PyFloat_Check/PyUnicode_Check/PyBytes_Check instead of *_CheckExact. Fixes user-defined int/str/bytes/float subclasses that were silently rejected with TypeError. Switched PyBytes_GET_SIZE to PyBytes_Size for subclass-safe length. 3. GIL release in DAE loop: SQLParamData and SQLPutData now release the GIL during each ODBC call, matching slow-path concurrency for large blobs/strings. 4. Preserve exec_rc: stash the SQLExecute return code before SQLFreeStmt so SUCCESS_WITH_INFO and other non-success-non-error codes are not clobbered by the unbind call. 5. Shallow-copy params: params = py::list(params) at function entry so DetectParamTypes' in-place PyList_SET_ITEM cannot mutate the caller's list under any future code path that might pass it directly. 6. Cursor attrs: SQLSetStmtAttr(SQL_ATTR_CURSOR_TYPE/CONCURRENCY) at entry to match slow-path semantics regardless of prior hstmt state. Also adds tests/test_023_fast_path_parity.py covering int/str/bytes/float subclasses, caller-list non-mutation, and unsupported-type TypeError.
Eight follow-up fixes after review feedback on c5a827f. 1. Refcount leak (BLOCKER): replace PyList_SET_ITEM (uppercase, no decref of old slot) with PyList_SetItem (decrefs old slot before stealing the new reference) in DetectParamTypes time/Decimal/UUID branches. The previous shallow-copy defense via py::list(params) was a no-op because pybind11s list constructor only inc_refs an already-list argument. 2. Geometry + DAE conflict: gate the geometry-prefix override on the not-DAE branch so a long POLYGON/POINT/LINESTRING string does not end up with isDAE=true, dataPtr set, AND a non-zero columnSize. 3. Decimal NaN/Infinity: throw ValueError instead of silently binding 0 via build_numeric_data on an empty digits tuple. 4. Time format: always emit microseconds (HH:MM:SS.ffffff), matching slow path isoformat(timespec=microseconds). 5. PyObject_IsInstance: explicit equality check so a custom __instancecheck__ that raises (returns -1) does not fall through with a Python error set. 6. Dead code: removed unused SMALLMONEY_MIN/SMALLMONEY_MAX constants and the unused utf16Len assignments in DetectParamTypes. 7. Encoding-key contract: only honor encoding_settings encoding when the user explicitly opted in via setencoding(..., ctype=SQL_C_CHAR=1). The Python layer SQL_C_CHAR constant is numerically -8 (real ODBC SQL_C_WCHAR), so by default the wide-char path is taken and encoding is irrelevant. 8. Parity test rewrite: drop the dead _force_slow_path_roundtrip helper, use the project cursor fixture instead of a hard-coded conn string, and add (a) a real fast-vs-slow parity check via setinputsizes-forced slow path, (b) a refcount-leak regression test using a Decimal subclass + weakref, (c) explicit NaN-rejection coverage.
Resolve conflicts in ddbc_bindings.cpp from main's GH-610 work: - Keep both build_numeric_data (this PR) and ResolveNullParamType (main) - Adopt main's BindParameters/BindParameterArray signatures that take SqlHandle& handle; update the SQLExecuteFast_wrap call site to pass *statementHandle so the fast path uses the per-handle NULL describe cache - Migrate SQLExecuteFast_wrap from std::wstring + WStringToSQLWCHAR to std::u16string + reinterpretU16stringAsSqlWChar (main's uniform 16-bit query/param representation), dropping the platform #ifdef in both the prepare path and the DAE wide-char put-data loop Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Honor use_prepare flag (was silently ignored, always preparing) - Move DetectParamTypes before SQLPrepare to prevent half-prepared state - Fix bytearray DAE crash (pybind11 bytes caster doesn't handle bytearray) - Replace lossy double MONEY comparison with exact Decimal arithmetic - Add SMALLMONEY range detection (was missing from fast path) - Handle PyObject_IsInstance error return (-1) with proper exception propagation - Clear describe cache on prepare (matching slow path) - Add edge case tests: large bytearray/bytes/string DAE, MONEY boundaries, Infinity rejection, embedded nulls Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ny-perf-detect-types
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace pybind11 .attr()/.cast<>() with raw CPython calls throughout
DetectParamTypes and build_numeric_data
- datetime/date/time: use PyDateTime_Check/PyDate_Check/PyTime_Check macros
and PyDateTime_TIME_GET_* accessors (requires PyDateTime_IMPORT)
- Decimal: PyObject_CallMethod/GetAttrString/RichCompareBool instead of
py::module_::import + py::object .attr() chains
- UUID: PyObject_GetAttrString("bytes_le") instead of py::handle .attr()
- Cache MONEY/SMALLMONEY Decimal bounds in PythonObjectCache (constructed
once at init, not per-call) using cached Python-side constants
- Replace magic int range numbers with UINT8_MAX/INT16_MIN/MAX/INT32_MIN/MAX
- Proper Py_DECREF cleanup on all error paths in build_numeric_data
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ecuteLegacy The new C++ pipeline is the primary path (99% of calls). The old function is the legacy fallback for setinputsizes users only. Naming should reflect this. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removes unnecessary pybind11 ↔ CPython round-trips in the hot path: - PythonObjectCache types stored as PyObject* (not py::object) - ParamInfo::dataPtr is raw PyObject* with explicit refcount management - DetectParamTypes takes PyObject* directly (not py::list&) - build_numeric_data returns NumericData struct (not py::object) - Added contextual comments explaining non-obvious design decisions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ility pybind11's type_caster needs copy semantics for std::vector<ParamInfo>& in the legacy path. Provide a copy ctor that Py_XINCREFs dataPtr. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Strings with embedded NUL characters (e.g., 'hello\x00world') were truncated at the first NUL because BindParameters used SQL_NTS (null-terminated string indicator). Now passes the actual byte/char length so ODBC sees the full string. Fixes test_string_with_embedded_nulls on all platforms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add tests: integer overflow (2**63), Decimal NaN/sNaN, precision > 38 - Add LCOV_EXCL markers on CPython import-failure and cache-fallback paths - Add contextual comments on PythonObjectCache and ParamInfo operators Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR shifts the primary cursor.execute() pipeline from Python into native C++ by adding a raw-CPython DetectParamTypes fast path and routing calls to a single DDBCSQLExecute FFI entrypoint when setinputsizes isn’t active, targeting large parameter-count performance regressions (GH-500).
Changes:
- Added a native
DetectParamTypes → BindParameters → SQLExecutepipeline (DDBCSQLExecute) to avoid per-parameter Python/pybind11 overhead. - Preserved a legacy path (
DDBCSQLExecuteLegacy) forsetinputsizesusers and updated Python routing accordingly. - Added parity and regression tests covering fast/slow path equivalence, subclass handling, DAE streaming, and refcount safety.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/test_023_fast_path_parity.py | Adds fast-vs-legacy parity tests and regression coverage for the new native execute pipeline. |
| tests/test_010_pybind_functions.py | Updates exposed-function expectations to include DDBCSQLExecuteLegacy. |
| mssql_python/pybind/ddbc_bindings.cpp | Implements native type detection, new execute entrypoints, and various binding/DAE handling updates. |
| mssql_python/cursor.py | Routes execute() to DDBCSQLExecute for the primary path and to DDBCSQLExecuteLegacy when setinputsizes overrides are present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…E safety - Check PyList_SetItem return value at all 4 call sites in DetectParamTypes - Copy mutable bytearray into std::string before DAE streaming (both paths) - Revert LCOV_EXCL markers (not processed by llvm-cov pipeline) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add PyTuple_Check guard before PyTuple_GET_SIZE/GET_ITEM on Decimal.as_tuple().digits in DetectParamTypes and build_numeric_data - Add ParamResetGuard RAII struct to ensure SQLFreeStmt(SQL_RESET_PARAMS) fires on all exit paths after BindParameters succeeds - Wrap PythonObjectCache::initialize() in try/catch to clean up partial refs on any import failure Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ParamResetGuard called SQLFreeStmt(SQL_RESET_PARAMS) in its destructor before the caller could read SQLGetDiagRec, producing empty SQLSTATEs. Restore manual SQLFreeStmt on success-only paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…_class, import_attr - PyObjGuard: RAII cleanup for Decimal tuple extraction in DetectParamTypes and build_numeric_data (eliminates ~15 manual decref cascades) - stream_dae_chunks(): template replacing 6 identical DAE chunking loops across legacy and fast execute paths - get_cached_class(): single helper replacing 5 copy-paste type getter functions - import_attr(): consolidates import-module-getattr-decref pattern in PythonObjectCache::initialize() Net -94 lines, no functional changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Explain the SQL_NUMERIC_STRUCT conversion algorithm step-by-step, precision/scale computation logic, MONEY range check rationale, and one-liners on helper utilities (PyObjGuard, stream_dae_chunks, get_cached_class). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Introduce py_ref.hpp: PyPtr = std::unique_ptr<PyObject, PyDecRefDeleter> with adopt() and incref_borrow() helpers. Zero runtime overhead via empty-base optimisation. Replaces the bespoke PyObjGuard (fixed 8-slot array, manual track/release) with standard C++ RAII throughout DetectParamTypes and build_numeric_data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… bewithgaurav/insertmany-perf-detect-types
…le-free bugs - Extract PythonObjectCache namespace to python_object_cache.hpp - Extend PyPtr usage to Groups A/B/D/E (22→7 manual decrefs) - Fix 3 double-free bugs in PyList_SetItem error paths - Document ParamInfo.dataPtr manual refcounting rationale Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename python_object_cache.hpp → py_type_cache.hpp - Rename namespace PythonObjectCache → PyTypeCache - Remove unused: incref_borrow using, <cctype>, <iomanip> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…om PyPtr drops py_ref.hpp (PyPtr = unique_ptr<PyObject, PyDecRefDeleter>) and uses pybind11's own RAII handle via a steal() shorthand. same ownership semantics, no behavior change: in release builds (-O3 -DNDEBUG, which is what we ship) py::object::dec_ref compiles to a bare Py_XDECREF, identical to the PyPtr deleter. the raw CPython calls in the hot detection path are untouched, only the refcount wrapper changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… bigint build_numeric_data walked every Decimal digit through PyNumber_Multiply/PyNumber_Add, allocating three PyLongs per digit, then called PyNumber_Absolute and to_bytes(16) to serialise. it also re-entered Python for as_tuple() and the digits/exponent attributes that DetectParamTypes had already fetched. SQL Server caps NUMERIC at 38 digits and callers reject anything larger, so the mantissa always fits 128 bits. accumulate it in four uint32 limbs and write the 16 little-endian bytes directly. limbs rather than __int128 because MSVC has no __int128, and the bytes are written explicitly so host endianness does not matter. as_tuple/digits/exponent are now passed in from the caller. numeric decimal path measured 2.3x-2.9x faster (19-digit 1643 -> 711 ns/param, 38-digit 2401 -> 824). money path and all non-decimal types unchanged. 1922 tests pass, and decimal round-trip verified across sign, zero, money bounds, positive exponents, 38-digit magnitudes and scale-38 values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…led rule of five ParamInfo kept dataPtr as a raw PyObject* and hand-wrote a destructor, copy constructor, copy assignment, move constructor and move assignment to pair Py_XINCREF with Py_XDECREF. five special members existed only to keep one refcount straight, and the copy assignment decref'd the old value before increfing the new one, which is the wrong order if the two are ever the same object. py::object already owns a refcount correctly. holding dataPtr as py::object makes the compiler-generated destructor, copy and move all correct, so the entire rule-of-five block goes away and the struct needs no special members at all. 70 lines deleted, 10 added. the two DetectParamTypes sites drop their explicit Py_INCREF, the two SQLParamData consumers read .ptr(), and the pybind property getter returns the object directly instead of reborrowing it. destruction still runs with the GIL held: the gil_scoped_release scopes in both execute paths close before the ParamInfo values die. 1922 tests pass, the refcount harness shows zero drift over 300 executes on every parameter case including str_long_dae, and the DAE path round-trips all types exactly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… call site steal() wrapped py::reinterpret_steal but had no counterpart, so the file mixed a 6-call shorthand against 8 spelled-out py::reinterpret_borrow<T>(py::handle(x)) calls. the dataPtr change in the previous commit added two more of the long form, so the asymmetry was growing. both helpers are now templated on the target type with py::object as the default, matching nanobind's nb::steal and nb::borrow signatures. the template is not cosmetic: four of the eight borrow sites need py::str or py::bytes rather than py::object, so a fixed-return helper would have covered only half of them. existing steal() calls are unaffected by the default argument. having the pair side by side also documents the hazard. steal on a borrowed reference (PyList_GetItem, PyTuple_GetItem, PyDict_GetItem) is a premature decref and a use-after-free, and that precondition now sits on the declaration instead of being implied by the name. no behavior change. 1922 tests pass and the refcount harness reports zero drift over 300 executes on all 15 parameter cases. all eight converted sites live in the DAE streaming paths of SQLExecuteLegacy_wrap and SQLExecute_wrap, so those were exercised directly: 7 DAE cases spanning NVARCHAR(MAX), VARCHAR(MAX) and VARBINARY(MAX), plus bytearray and the 4001-unit boundary, all round-trip byte-exact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
py_ref.hpp existed earlier in this branch as the home for the custom PyPtr wrapper, and was deleted when PyPtr was replaced by py::object. steal() had to land somewhere, so it went into py_type_cache.hpp, which was already using it. borrow() then followed it there. neither belongs in that file: its own first line describes it as a cache of Python type objects and MONEY boundary constants, and these two helpers are neither. restores py_ref.hpp with the reference-adoption helpers and nothing else, and gives py_type_cache.hpp back a description that matches its contents. only ddbc_bindings.cpp includes either header, so the include change is one line. no behavior change. rebuilt and the .so is byte-identical to the previous commit (sha256 43ec909d...), with ddbc_bindings.cpp recompiled and relinked rather than served from cache. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
this PR added roughly 430 lines of new parameter-detection code into ddbc_bindings.cpp, a file that was already 6600 lines. DetectParamTypes, build_numeric_data and the types they produce are the first stage of the execute pipeline and are cohesive enough to read on their own, so they now live in their own header. ddbc_bindings.cpp drops to 6064 lines and only the pieces this PR introduced moved, so no pre-existing code shifts and no other in-flight branch gains a conflict. the SQL Server ODBC constants that sql.h does not expose move from ddbc_bindings.cpp up into ddbc_bindings.h, because both the detection path and the fetch paths need them and the header is included before either. header rather than .cpp on purpose. the build is -O3 with no LTO, so a .cpp boundary is also an inlining boundary, and these helpers run once per parameter per execute. defining them inline in a header keeps them in the including translation unit. once LTO is enabled this can become a normal .cpp. the resulting binary is not quite bit-identical and the reason is worth stating: __text grows 92 bytes and DetectParamTypes gains an out-of-line symbol. previously it sat in an anonymous namespace with exactly one call site, so the compiler inlined it into SQLExecute_wrap and deleted the original; as an inline function with vague linkage it is now emitted once and called. that is one call per execute(), not per parameter, against a roughly 300us execute. build_numeric_data, which does run per decimal parameter, was already out-of-line before this change and still is: the only difference in its symbol is the mangled name losing the anonymous-namespace prefix. no other symbol changed. 1922 tests pass, the refcount harness reports zero drift across all 15 parameter cases over 300 executes, and the 7 DAE round-trip cases remain byte-exact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| "NumericData valueBytes size exceeds SQL_MAX_NUMERIC_LEN (16)"); | ||
| } | ||
| // Copy binary data to buffer, remaining bytes stay zero-padded | ||
| std::memcpy(&val[0], valueBytes.data(), valueBytes.size()); |
…oval the two execute entry points are DDBCSQLExecute and DDBCSQLExecuteLegacy, so the surrounding code should read standard versus legacy. cursor.py still called the non-legacy branch use_fast_path, which named a third thing that does not exist and left the reader guessing which C++ function it reached. renamed to use_standard_execute, and the parity test file follows: test_023_fast_path_parity.py becomes test_023_execute_path_parity.py, with _fast_path_roundtrip becoming _standard_roundtrip. every legacy site now says out loud that it is temporary and why it still exists. the legacy branch survives only for setinputsizes() callers, whose explicit type overrides the native path does not yet honour; that is the single thing blocking its removal, and it was not written down anywhere. annotated in cursor.py at the branch and the call, on SQLExecuteLegacy_wrap, on the DDBCSQLExecuteLegacy binding, and on the PyTypeCache import fallback that exists only because the legacy path can run before the cache is warm. _create_parameter_types_list gets a fuller docstring rather than a removal note, because it has two callers and only one of them is legacy: executemany() still needs it and will keep needing it until columnwise detection is native too. calling it simply legacy would have been wrong. left alone: the 'Fast path: Data fits in buffer' comments in ddbc_bindings.h and the ASCII-prefix fast path in test_002 and test_014. same words, unrelated concept, pre-existing. comments and identifiers only, no logic touched. 1922 tests pass and the renamed parity file runs all 51 of its tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| def test_decimal_precision_overflow_rejected(cursor): | ||
| """Decimals beyond SQL Server's max precision must raise.""" | ||
| with pytest.raises(Exception): | ||
| cursor.execute("SELECT ?", [decimal.Decimal("123456789012345678901234567890123456789")]) |
Work Item / Issue Reference
Summary
Moves parameter type detection and binding from Python into a native C++ pipeline using raw CPython API calls. The new
DDBCSQLExecutehandles type detection → parameter binding → SQLExecute in a single FFI crossing, eliminating per-parameter Python overhead entirely.What changed:
DetectParamTypes— C++ type detection using raw CPython API (PyLong_Check,PyDateTime_Check,PyObject_RichCompareBool, etc.) replacing the Python-side_create_parameter_types_listloop for the primary execute path.DDBCSQLExecute(formerlyDDBCSQLExecuteFast) — single C++ pipeline: detect → bind → execute. ParamInfo never crosses the pybind11 boundary.DDBCSQLExecuteLegacy(formerlyDDBCSQLExecute) — retained forsetinputsizesusers only.PythonObjectCachestores all type objects as rawPyObject*(notpy::object), eliminating pybind11 wrapper overhead on every cache hit.Py_DECREFcleanup andPyObject_IsInstanceerror handling on all paths.Routing (cursor.py):
Performance Results 🚀
The Python-side type detection cost was ~2.0–2.3µs per parameter — an
isinstancecheck,ParamInfoobject construction, and a pybind11 FFI boundary crossing per parameter, per execute call. The C++ path replaces this with ~35ns/param (rawPyLong_Check+ struct field write) — a ~60x faster per-parameter detection.macOS arm64 (Apple Silicon M-series), Python 3.13
Linux aarch64 (Docker container), Python 3.13
vs pyodbc (post-PR, macOS)
Bottom line
Checklist